Draft jobs table: Show caution for older data#3703
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3703 +/- ##
=======================================
Coverage 81.74% 81.74%
=======================================
Files 619 619
Lines 38663 38668 +5
Branches 6297 6322 +25
=======================================
+ Hits 31606 31611 +5
Misses 6096 6096
Partials 961 961 ☔ View full report in Codecov by Sentry. |
Nateowami
left a comment
There was a problem hiding this comment.
@Nateowami reviewed 4 files and all commit messages, and made 4 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on marksvc).
.github/copilot-instructions.md line 117 at r1 (raw file):
# Tests - Place a line with "// SUT" before the line that causes the code being tested to be exercised.
Can we add conditions and/or caveats to this? A lot of tests are simple enough that this would just be noise. Right now we have 168 instances of ``// SUTin*.ts` files, and 2,745 instances of `it(`. I'm sure some could use it that lack it, but I'm not sure a 16-fold increase in usage is called for.
src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.ts line 214 at r1 (raw file):
/** Whether to show a caution notice about comparing older durations. */ get shouldShowDurationComparisonCaution(): boolean {
As a nit about naming, I think should is mostly redundant. That said, I do appreciate erring on the side of calrity, and not choosing a name like isDurationComparisonCuationShown.
src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.ts line 219 at r1 (raw file):
// Day after the date when draftGenerationRequestId began to be used, which was in SFv5.46.1 at // 2025-12-18T17:48:57Z. const requestIdIntroductionDate: Date = new Date(2025, 12 - 1, 18 + 1);
This would be way simpler as new Date(2025-12-18) (or you could use an exact timestamp) since everyone is more familiar with ISO date formats than JavaScript date APIs. Additionally, it's interpreted as UTC, whereas constructing a date from numbers results in it being interpreted as local time, meaning the cutoff depends on the user's timezone.
(Also in tests)
src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/draft-jobs.component.ts line 222 at r1 (raw file):
const rangeStartMs: number = this.currentDateRange.start.getTime(); const cutoffMs: number = requestIdIntroductionDate.getTime();
For what it's worth, you can directly compare JavaScript datetimes (and if you couldn't, TypeScript would yell at you for trying to compare incompatible types).
JavaScript months are zero indexed. Rather than write
11for December, I wrote12 - 1.Screenshot:
This change is